edns: allow dnsconfd name resolution for kickstart fetching - #6258
Conversation
|
/build-image |
|
Images built based on commit eada098:
Download the images from the bottom of the job status page. |
ec4789b to
11341a5
Compare
|
/build-image |
|
/kickstart-test dns-global-bootopts dns-global-exclusive-tls-2 dns-global-exclusive-tls-httpks-2 dns-global-exclusive-tls-httpks dns-global-exclusive-tls-initramfs dns-global-exclusive-tls-ksnet dns-global-exclusive-tls dns network-dns-search |
|
Images built based on commit 11341a5:
Download the images from the bottom of the job status page. |
|
/kickstart-test --force dns-global-bootopts dns-global-exclusive-tls-2 dns-global-exclusive-tls-httpks-2 dns-global-exclusive-tls-httpks dns-global-exclusive-tls-initramfs dns-global-exclusive-tls-ksnet dns-global-exclusive-tls dns network-dns-search |
|
/build-image |
|
Images built based on commit b08f034:
Download the images from the bottom of the job status page. |
b08f034 to
f1b567e
Compare
|
/kickstart-test --testtype dns |
f1b567e to
e4e0bdd
Compare
|
/kickstart-test --testtype dns |
e4e0bdd to
dd71040
Compare
|
/build-image |
|
Images built based on commit dd71040:
Download the images from the bottom of the job status page. |
dd71040 to
c7249d1
Compare
|
/kickstart-test --testtype smoke |
|
/kickstart-test --testtype dns |
c7249d1 to
5e56c15
Compare
5e56c15 to
4e70724
Compare
Start dnsconfd already before kickstart fetching. We used to start dnsconfd only after the kickstart was fetched if kickstart usage was detected so that the potential certificates from kikcstart are applied. But this mechanism ruled out use case when dnsconfd (name resolution) is needed for the kickstart fetching. So start the dnsconfd early and if certificates were fetched restart it (it is done by restarting unbound service as recommended by dnsconfd). Resolves: RHEL-82694
We need this guard because kickstart may be fetched over multiple network devices and it is parsed after each fetching. Parsing itself should be idempotent (multiple parsing harmless) as it only dumps files with configuration / options for further actions. Resolves: RHEL-82694
4e70724 to
d98e411
Compare
|
/kickstart-test --testtype dns |
|
/kickstart-test --testtype smoke |
There was a problem hiding this comment.
Hey @rvykydal - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding more comments to explain the logic behind the different stages in
start_dnsconfd. - It might be helpful to define the possible values of the
stagevariable as constants to improve readability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jkonecny12
left a comment
There was a problem hiding this comment.
Looks good to me.
I just wonder if these string constants can't be translated to something more robust. Something like start_dnsconfd <should import certificates yes/no> <reason>
The approach here is to keep all the logic intentionally at single place - in the function. The callers are only passing information where the call comes from. |
jstodola
left a comment
There was a problem hiding this comment.
It seems to work fine, thanks.
https://issues.redhat.com/browse/RHEL-82694